CA-406953: fix more undefined behaviour#21
Conversation
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Otherwise it warns that it is a non-literal and can't type check it. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
The `v` was missing, and instead of calling the varargs version of printf, it called the regular one. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
edwintorok
force-pushed
the
private/edvint/undef2
branch
2 times, most recently
from
addf587 to
1c274ea
Compare
andyhhp
left a comment
andyhhp
left a comment
There was a problem hiding this comment.
Strictly speaking, the use of PMTC_S8 for format strings is undefined, because char has implementation-defined signed-ness. (Unlike other types in C, char and signed char do not mean the same thing.)
But, it's probably not worth boiling that particular ocean
Fixes: ``` /usr/include/x86_64-linux-gnu/bits/fcntl2.h:50:11: error: call to ‘__open_missing_mode’ declared with attribute error: open with O_CREAT or O_TMPFILE in second argument needs 3 arguments ``` Signed-off-by: Edwin Török <edwin.torok@cloud.com>
edwintorok
force-pushed
the
private/edvint/undef2
branch
2 times, most recently
from
d7ee2d6 to
1ca75ac
Compare
Do not rely on size of 'int' and 'long long', although they happened to work in this case. Use stdint.h types instead. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Sometimes a format string with wrong signedness, or wrong size was used. The wrong size is probably undefined behaviour. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
edwintorok
force-pushed
the
private/edvint/undef2
branch
from
1ca75ac to
26a80ad
Compare
lib/weightio.c
Outdated
| open_hostweight_file(int *fd, int *err_no) | ||
| { | ||
| if ((*fd = open(HA_HOST_WEIGHT_FILE, O_RDWR|O_CREAT)) < 0) | ||
| if ((*fd = open(HA_HOST_WEIGHT_FILE, O_RDWR|O_CREAT, 00400)) < 0) |
There was a problem hiding this comment.
Minor, typo, one leading zero is enough.
| PMTC_S8 log_string, | ||
| MTC_HOSTMAP hostmap); | ||
| MTC_HOSTMAP hostmap) | ||
| __attribute__((format(printf, 2, 0))); |
There was a problem hiding this comment.
Are you sure? I cannot see the parameters, how can be a formatting string? Maybe it's a bug?
There was a problem hiding this comment.
Okay, never mind, the 0 says there are no parameter to check against. Maybe a kind of trick can be in place like
static inline const char *
check_print_liveset_format(const char *fmt, ...) __attribute__((format(printf, 1, 2))
{
return fmt;
}
#define print_liveset(pri, fmt, map) print_liveset(pri, check_print_liveset_format(fmt, "dummy"), map)There was a problem hiding this comment.
The compiler is still not entirely happy about it, because it says that the 'log_string' is not a string literal, whereas in the other function where I used format(printf,2,3) it was able to see that fmt is a string literal from the caller and didn't warn about it again.
I'll try the macro trick, that should fix it.
5 participants
When O_CREAT is used then the file mode must be specified, otherwise it'll be something random from the stack:
The file is opened read/write so set matching permission bits.
In
command/stubs.c(used bywritestatefile) we need to callvfprintfwithap, instead offprintf.fprintfwould expect the actual arguments, whereasvfprintfwill forward the varargs correctly.